-
-
Notifications
You must be signed in to change notification settings - Fork 258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve UnsatisfiableInterpreterConstraintsError. #1028
Improve UnsatisfiableInterpreterConstraintsError. #1028
Conversation
Surface all broken interpreters and a link to known breaks and workarounds tracked in pex-tool#1027.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I like this compromise.
This is good. I would like to see this link to the pants.readme.io documentation instead of a GitHub issue. The installation problems should live near the installation documentation. |
Those docs are for Pants, not Pex. Pex docs are unfortunately extremely stale, as we don't have the keys to the site anymore: https://pex.readthedocs.io/en/stable/. Benjy and I have discussed a 10% side project for me to add a dedicated Pex site for pants.readme.io. It would help Pex users and, by extension, Pants users. |
Understood. I was conflating the two sites then. Separately, we should recover access to the pex site. Seems like support is done through GitHub issues: https://docs.readthedocs.io/en/stable/contribute.html#initial-triage |
Somehow id(PythonInterpreter) != id(cls) on Travis interpreters.
a9a448f
to
ae95ab3
Compare
def _compatible_interpreters_iter(interpreters_iter, compatibility_constraints=None): | ||
if compatibility_constraints: | ||
for interpreter in matched_interpreters_iter(interpreters_iter, compatibility_constraints): | ||
for interpreter in _matched_interpreters_iter(interpreters_iter, compatibility_constraints): | ||
yield interpreter | ||
else: | ||
for interpreter in interpreters_iter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This branch does not handle the switch from PythonInterpreter.iter
to PythonInterpreter.iter_candidates
here: https://github.com/pantsbuild/pex/pull/1028/files#diff-670bc388096d1ef0c7994c4d37976392R82 so we can get error tuples mixed into the yielded interpreters when there are no compatibility constraints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we can get error tuples
This was something I was confused by when adding type hints. In addition to the type hints, we may want to rename this to interpreter_or_failure
:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is better fixed by restructuring for locality. The code that calls PythonInterpreter.iter_candidates should deal with the consequences of that directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not rename since we don't use Hungaian elsewhere but this confusing bit of code where typing is path-dependent is now typed in #1046
Surface all broken interpreters and a link to known breaks and
workarounds tracked in #1027.